Reviewing files that changed from the base of the PR and between b44d836 and 6e81094.

• pkg/connector/workspace.go

pkg/connector/workspace.go (2)

21-21: businessPlusClient field is still actively used and should not be removed.

The businessPlusClient is actively used throughout the codebase for SCIM API operations including:

• User listing (user.go)
• IDP group listing and management (group.go)
• User enable/disable actions (actions.go)

Removing it from the Grants method does not make the field itself unnecessary.

Likely an incorrect or invalid review comment.

---

141-144: Fix compilation error: GetUsers() requires context and options parameters.

Line 141 calls o.client.GetUsers() with no arguments, but the slack-go library's GetUsers() method requires context and options. The correct call pattern is GetUsersContext(ctx, options) as shown in user.go.

Additionally, this change removes pagination support. The Slack API's users.list endpoint returns paginated results, and large workspaces may not receive all users in a single call without handling the pagination cursor.

Replace the call with:

options := slack.GetUsersOptionTeamID(resource.Id)
users, err := o.client.GetUsersContext(ctx, options)

This preserves pagination handling and matches the pattern used in user.go.

⛔ Skipped due to learnings

Learnt from: mateoHernandez123
Repo: ConductorOne/baton-slack PR: 57
File: pkg/connector/roles.go:217-219
Timestamp: 2025-10-28T17:48:53.644Z
Learning: In the baton-slack connector, when client methods like SetWorkspaceRole(), RemoveUser(), or other enterprise/slack client calls already wrap errors with specific gRPC codes via handleError(), use fmt.Errorf with %w to preserve those codes while adding context. Only use uhttp.WrapErrors for errors generated directly in the function (e.g., validation, precondition checks). Double-wrapping with uhttp.WrapErrors would overwrite the specific codes (Unavailable, PermissionDenied, etc.) needed for proper retry logic and alerting.
<!-- [/add_learning]